Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
sayakpaul
left a comment
sayakpaul
left a comment
There was a problem hiding this comment.
I left some comments. Let's also add some simple tests:
- Checking if the params are being serialized (just checking if the checkpoint files exist is enough)?
modular_model_index.jsonreflects the repository locations properly?
| If expressed as an integer, the unit is bytes. | ||
| push_to_hub (`bool`, *optional*, defaults to `False`): | ||
| Whether to push the pipeline to the Hugging Face model hub after saving it. | ||
| **kwargs: Additional keyword arguments passed along to the push to hub method. |
There was a problem hiding this comment.
Should we document what is allowed in the kwargs? overwrite_modular_index deserves some documentation IMO.
| if component_spec.default_creation_method != "from_pretrained": | ||
| continue |
There was a problem hiding this comment.
Could you explain what this is doing?
|
|
||
| # Create a new empty model card and eventually tag it | ||
| if push_to_hub: | ||
| card_content = generate_modular_model_card_content(self.blocks) |
There was a problem hiding this comment.
Is this conditioned on the above changes? If not, maybe we can keep it in the earlier position?
|
|
||
| save_method(os.path.join(save_directory, component_name), **save_kwargs) | ||
|
|
||
| if push_to_hub: |
There was a problem hiding this comment.
Would prefer to fully push the push_to_hub path to the end of the implementation as it's easier to follow.
| private = kwargs.pop("private", None) | ||
| create_pr = kwargs.pop("create_pr", False) | ||
| token = kwargs.pop("token", None) | ||
| repo_id = kwargs.pop("repo_id", save_directory.split(os.path.sep)[-1]) |
| repo_id = kwargs.pop("repo_id", save_directory.split(os.path.sep)[-1]) | ||
|
|
||
| for component_name, component_spec in self._component_specs.items(): | ||
| sub_model = getattr(self, component_name, None) |
There was a problem hiding this comment.
(nit):
| sub_model = getattr(self, component_name, None) | |
| component = getattr(self, component_name, None) |
Not all components need to models.
| if variant is not None and "variant" in component_spec_dict: | ||
| component_spec_dict["variant"] = variant | ||
|
|
||
| self.register_to_config(**{component_name: (library, class_name, component_spec_dict)}) |
There was a problem hiding this comment.
I am not too sure about the objective of this block. What happens if its corresponding model_cls doesn't have the save method we support through LOADABLE_CLASSES?
Or is this unrelated?
3 participants
What does this PR do?
Enable modular pipelines to save model weights to the Hub. Previously, only configs were saved. Note that if individual components are loaded from external repos, their model configs will continue to reference those repos. I've added an optional
overwrite_modular_indexflag to rewrite component configs so they point to the destination repo instead.Fixes # (issue)
Before submitting
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.